-
Notifications
You must be signed in to change notification settings - Fork 47
ir: move config options out of top level #3619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ir: move config options out of top level #3619
Conversation
da78f7f
to
aef9c30
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3619 +/- ##
==========================================
+ Coverage 26.75% 26.79% +0.04%
==========================================
Files 653 653
Lines 51047 51111 +64
==========================================
+ Hits 13656 13697 +41
- Misses 36337 36362 +25
+ Partials 1054 1052 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CHANGELOG.md
Outdated
`neofs-adm fschain estimations` was removed. | ||
|
||
Use new `fschain.disable_autodeploy` IR configuration option instead of deprecated `fschain_autodeploy`, | ||
that was removed in next release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mention that default value for the old config has changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but in fact, i think it is useless work, we will just drop it in the next release, nobody gonna change this exact value in their configs, they will just use the new one
pkg/innerring/innerring.go
Outdated
|
||
serveMetrics(server, cfg) | ||
|
||
// TODO: remove deprecated option in future releases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue?
pkg/innerring/innerring.go
Outdated
serveMetrics(server, cfg) | ||
|
||
// TODO: remove deprecated option in future releases | ||
if cfg.IsSet("fschain_autodeploy") && cfg.IsSet("fschain.disable_autodeploy") && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think any case when both values are set can be reported as an error, this is smth abnormal to me
aef9c30
to
1aff241
Compare
fschain_autodeploy
out of top level1aff241
to
6f64aae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems good
pkg/innerring/innerring.go
Outdated
if cfg.IsSet("fee.main_chain") { | ||
log.Warn("configuration option 'fee.main_chain' is deprecated, use 'mainnet.extra_fee' with the same value instead") | ||
// default value | ||
if cfg.Mainnet.ExtraFee == 50000000 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default value may be specified in the config file explicitly. This will mutate it. We should be able to distinguish this case from Viper.SetDefault()
, and return an error
if cfg.IsSet("fschain_autodeploy") && cfg.IsSet("fschain.disable_autodeploy") { | ||
return nil, fmt.Errorf("'fschain_autodeploy' and 'fschain.disable_autodeploy' set simultaneously") | ||
} | ||
if cfg.IsSet("fschain_autodeploy") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be merged with the previous if
. Not a big deal taking into account further removal of both blocks
6f64aae
to
d8d0330
Compare
d8d0330
to
0c9d9d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebase, please.
Don't use viper's `SetDefault`, because it triggers configs `Set` and then we cannot determine whether this value was in the configuration or not. Signed-off-by: Andrey Butusov <[email protected]>
New `fschain.disable_autodeploy` IR configuration, which inverts the value of the `fschain_autodeploy` option. Make `fschain_autodeploy` default to `true`, support it for one more release. Closes #3361. Signed-off-by: Andrey Butusov <[email protected]>
0c9d9d1
to
bf59214
Compare
`mainnet.enabled` IR configuration, which inverts the value of the `without_mainnet` option. By default, mainnet is disabled now. Ref #3362. Signed-off-by: Andrey Butusov <[email protected]>
Ref #3362. Signed-off-by: Andrey Butusov <[email protected]>
Ref #3362. Signed-off-by: Andrey Butusov <[email protected]>
Closes #3362. Signed-off-by: Andrey Butusov <[email protected]>
Closes #3361, #3362.